In `@client/internal/peer/worker_ice.go`:
- Line 431: Add a brief comment above the line setting priority :=
srflxCandidate.Priority() + 1000 explaining why a fixed +1000 offset is applied
for port-forwarded candidates: state that the offset intentionally increases
priority to favor port-forwarded srflx candidates during connectivity checks,
note that +1000 is an arbitrary chosen boost value (or reference the design
doc/issue if applicable), and describe how this interacts with the RFC 8445
priority formula (i.e., it is an additional global boost on top of the
candidate.Priority() computed per RFC 8445); optionally mention that a future
change could recompute priority using
type-preference/local-preference/component-ID components instead of a fixed
offset.

In `@client/internal/portforward/manager_test.go`:
- Around line 142-151: TestState_Cleanup calls State.Cleanup which triggers real
NAT/gateway discovery (causing slow/flaky tests); stub out the discovery used by
Cleanup in this unit test (e.g., override the package-level discover function or
inject a fake gateway) to return a no-op gateway or nil mappings and restore the
original after the test so Cleanup runs deterministically and quickly; update
TestState_Cleanup to replace the discovery function before calling state.Cleanup
and to restore it in a defer.

In `@client/internal/portforward/manager.go`:
- Around line 54-76: The Start method currently casts wgPort to uint16 without
validation which can wrap invalid values; add a guard at the top of
Manager.Start to validate wgPort is between 1 and 65535 before assigning
m.wgPort: if out of range, log an error (using log.Errorf or log.Infof) with
context (include the wgPort value) and return without starting the mapper. Keep
the existing early return for isDisabledByEnv() and retain use of m.ctx,
m.cancel, m.stateManager.RegisterState and go m.run() only after the port check
passes.
- Around line 195-212: The Stop method on Manager currently cancels work and
persists state but doesn't remove the active port mapping; modify Manager.Stop
to perform a best-effort deletion of the port mapping before returning and to
clear persisted state so the mapping doesn't linger until TTL expiry.
Specifically, in Manager.Stop (which uses m.mu, m.cancel, m.wg) call the
existing mapping-removal routine (or implement a call like
deletePortMapping/removeMapping) after cancel() and wg.Wait() (or immediately
after cancel() if safe), handle and log any error as non-fatal, then clear
in-memory/persisted state by calling persistStateLocked to write an
empty/cleared state (or a new clearPersistedState function) while holding m.mu;
ensure m.cancel is nil and any mapping metadata is removed so subsequent
restarts don't re-persist the mapping.
- Around line 251-275: renewMapping updates m.mapping.ExternalPort but never
causes the ICE agent to refresh candidates, leaving injected server-reflexive
candidates stale; detect when uint16(externalPort) != m.mapping.ExternalPort in
renewMapping and, after updating m.mapping.ExternalPort, either reset the
port-forward state (clear portForwardAttempted) and call the same injection path
used in worker_ice.go (invoke injectPortForwardedCandidate or the routine that
creates/injects candidates), or trigger the ICE agent to re-gather (call the
agent's Gather/Restart method) so new candidates reflecting the updated external
port are signaled; update renewMapping to perform one of these actions and
ensure it uses the same ctx/locking semantics as the current function.

In `@go.mod`:
- Line 66: Update the two outdated NAT traversal dependencies: bump
github.com/huin/goupnp from v1.2.0 to v1.3.0 and github.com/koron/go-ssdp from
v0.0.4 to v0.1.0 in go.mod (or run go get github.com/huin/goupnp@v1.3.0
github.com/koron/go-ssdp@v0.1.0), then run go mod tidy to update the lockfile
and ensure builds pass; while here, confirm that github.com/libp2p/go-nat is
still the intended NAT provider for the project (as libp2p has moved NAT
functionality into go-libp2p) and adjust/remove it if necessary.

In `@client/internal/portforward/manager.go`:
- Around line 219-222: The early return after calling
m.gateway.DeletePortMapping leaves m.mapping set and skips persisting state;
change the flow so DeletePortMapping errors are logged but do not return
early—always clear m.mapping and call the manager's persistence routine (e.g.,
m.persist / m.saveState or the existing state-save method) so in-memory state
and on-disk state are consistent; easiest is to log the error from
m.gateway.DeletePortMapping(ctx, m.mapping.Protocol,
int(m.mapping.InternalPort)) and then continue to set m.mapping = nil and invoke
the state persistence method (or ensure a defer clears/persists) before
finishing.

In `@client/internal/portforward/manager.go`:
- Around line 153-185: createMapping (and similarly renewMapping) currently
holds m.mu while calling blocking NAT ops (AddPortMapping/GetExternalAddress),
preventing Stop() from acquiring the lock; fix by copying the minimal state
under lock (e.g., gateway, wgPort, mappingDescription, defaultMappingTTL),
release m.mu, perform the network calls with a cancellable ctx, then re-lock
m.mu and verify the gateway/state is still valid before updating m.mapping and
calling persistStateLocked(); ensure you follow the same pattern in renewMapping
and avoid calling AddPortMapping/GetExternalAddress while m.mu is held so Stop()
can promptly cancel the context.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@client/internal/peer/worker_ice.go`:
- Around line 379-395: The code sets w.portForwardAttempted too early; move the
assignment so it is only set after a valid mapping is available: first check
pfManager != nil and mapping := pfManager.GetMapping() != nil, then acquire
w.muxAgent lock and check/assign w.portForwardAttempted (to avoid races) and
release the lock; keep the early-return behavior if portForwardAttempted is
already true, but perform that check inside the locked section after confirming
mapping exists so an initial SRFLX candidate doesn't permanently disable port
forwarding. Reference symbols: w.muxAgent, w.portForwardAttempted, pfManager,
pfManager.GetMapping().

In `@client/internal/portforward/manager_test.go`:
- Around line 149-164: The test currently asserts the mapping is absent after
calling State.Cleanup() without first ensuring a mapping existed; update
TestState_Cleanup to seed the mock gateway with a mapping for the port under
test (e.g., set mockGateway.mappings[51820] = some mapping) and assert it exists
before calling state.Cleanup(), then call state.Cleanup() and assert the mapping
is removed; reference the mockGateway.mappings map and the State.Cleanup() call
so the test fails if deletion regresses.
- Around line 71-143: Update the tests to match the current Manager API: call
NewManager() with no arguments (replace NewManager(sm) with NewManager()), call
createMapping with the correct signature (e.g.,
m.createMapping(context.Background(), newMockNAT()) instead of
m.createMapping(nil)), and replace uses of the non-existent m.IsAvailable() with
explicit checks against the current API (for example assert availability by
testing m.GetMapping() != nil and m.gateway != nil or equivalent conditions).
Ensure references to Mapping, gateway, GetMapping and createMapping are used so
the tests compile against the existing Manager implementation.

In `@client/internal/portforward/manager.go`:
- Around line 239-250: Manager.cleanup leaves m.mapping populated after
successfully deleting the port on the gateway, so callers of GetMapping() can
return a stale mapping; update Manager.cleanup (the method named cleanup on type
Manager) to set m.mapping = nil after a successful gateway.DeletePortMapping
call (after the log.Infof or just before returning) so the in-memory state
reflects the remote deletion and GetMapping() no longer returns the removed
mapping.